Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose issue66 via unit test #84

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

maiqueb
Copy link
Collaborator

@maiqueb maiqueb commented Nov 17, 2022

What this PR does / why we need it:
This PR introduces a unit test that exposes the bug described in issue #66 .

It furthermore shows that updates to the MAC address (for instance) of an existing attachment do work as expected.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer (optional):
Depends-on: #83

This commit refactors the previously added test into a table that
clearly proves that updating the interface name of an existing
attachment actually causes the original attachment to be removed, since
this "update" is recognized by the controller as a sequence of:
- attach new interface (with the interface name specified)
- remove old interface

The first action fails, since that interface name is already present
within the system, while the second succeeds.

Signed-off-by: Miguel Duarte Barroso <[email protected]>
InterfaceRequest: "net0",
MacRequest: "07:06:05:04:03:02",
}),
XEntry("when the iface name is updated", nad.NetworkSelectionElement{
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong; the original attachment does feature the interface name.

We need to somehow have a pod with a provisioned attachment that does not specify the interface name on the network selection elements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant